ci(publish): fail the release if a published package cannot be imported#72
Open
aksumustafa1625 wants to merge 1 commit into
Open
ci(publish): fail the release if a published package cannot be imported#72aksumustafa1625 wants to merge 1 commit into
aksumustafa1625 wants to merge 1 commit into
Conversation
Two releases have now shipped artifacts that no consumer could import: salesforce#35 dist/ still referenced the internal @agentscript/* scope after the publish rewrite, so consumers hit ERR_MODULE_NOT_FOUND. salesforce#71 agentforce, compiler and agentforce-dialect were published pinning @sf-agentscript/language@2.5.4 while their dist/ had been compiled against a newer language, so consumers hit a SyntaxError for the missing export nullLiteralValidationPass and a TypeError for the missing variantMatch builder. Neither is visible from inside the workspace: pnpm resolves siblings from source, so the packed manifest is never exercised, and packing from the workspace always produces a self-consistent set of tarballs. The failure only appears once a dependent resolves a sibling from the registry, i.e. only in a real consumer install. This adds that install as a publish step. scripts/verify-published-packages.mjs installs each publishable package from the registry into a clean directory and imports its public entry point in a child process, skipping entries that are not bare-importable by design (lsp-server, lsp-browser, monaco, parser-tree-sitter). publish.mjs runs it after `changeset publish`, so a broken release fails the workflow in minutes instead of being found by the first user to install it. Verified against the currently published packages: it flags agentforce@2.5.34, compiler@2.7.1 and lsp@2.3.8 with their exact errors, and passes language@2.18.0, parser@4.0.4 and types@0.2.3. Refs salesforce#71
|
Thanks for the contribution! Before we can merge this, we need @aksumustafa1625 to sign the Salesforce Inc. Contributor License Agreement. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #71.
The problem
Two releases in a row have shipped artifacts that no consumer can import:
dist/still referenced the internal@agentscript/*scope after the publish rewrite →ERR_MODULE_NOT_FOUND.nullLiteralValidationPass) #71 —agentforce,compilerandagentforce-dialectwere published pinning@sf-agentscript/language@2.5.4while theirdist/had been compiled against a newerlanguage→SyntaxError: ... does not provide an export named 'nullLiteralValidationPass'andTypeError: ....variantMatch is not a function.Right now, on npm
latest, a bareimportof@sf-agentscript/agentforce,@sf-agentscript/compilerand@sf-agentscript/lspall fail.Neither bug is visible from inside the workspace, and no unit test can see them: pnpm resolves siblings from source, so the packed manifest is never exercised. The failure only appears once a dependent resolves a sibling from the registry — i.e. only in a real consumer install.
What this adds
scripts/verify-published-packages.mjs— for every publishable workspace package, install<name>@<version>from the registry into a clean temp directory the way a consumer does, thenimport()its public entry point in a child process. Any failure exits non-zero.scripts/publish.mjsruns it as step 7, right afterchangeset publish, so a broken release fails the workflow within minutes instead of being discovered by the first person who installs it.Design notes (happy to change any of these)
Why post-publish and not CI. I tried the pre-publish version first and it does not work: packing from the workspace always produces a self-consistent set of tarballs, so the stale pin resolves to the local sibling and the check passes while the published artifact is broken. The bug is only observable against the registry, so the check has to run against the registry.
Why
npmand notpnpmfor the consumer install.pnpm-workspace.yamlsetsminimumReleaseAge: 1440, which would refuse to install a package published seconds earlier.Skips. Four entry points are not bare-importable by design, and each skip reason was confirmed by actually importing the published package rather than assumed:
lsp-serverConnection input stream is not set…by designlsp-browserTypeError: port.addEventListener is not a functionin Nodemonacosrc/index.ts)parser-tree-sitterPackages whose version is not on the registry are skipped (e.g.
compile-server, which is publishable but has never been published).Verification
I could not build the monorepo locally to exercise the full publish path —
pnpm installfails ontree-sitterunder Node 24, which is #7. So I verified the check itself directly, against the packages that are broken on npm right now. The script takes explicit<name>@<version>specs for exactly this kind of spot-check:It flags all three broken packages with their exact errors and passes the three healthy ones — no false positives. Had this been in place, both #35 and #71 would have failed their release instead of shipping.
pnpm format:checkpasses on both files (ESLint only covers.ts/.tsx, so the.mjsscripts are not linted).Not included
This does not fix the stale pins themselves — that is a separate, one-line-per-package change, and you may prefer to fix the root cause in the release flow (a
languagerelease added API without a matching version bump for the dependents packed against it). This PR only makes sure that class of mistake can never reach users unnoticed again.Happy to move the step, rename the script, or widen/narrow the skip list — whatever fits how you want releases gated.